New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Logger has not export in entry file #1276
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
History in 2.0.0
It was changed from interface
to class
back in a1e40c2 which was released as 2.9.0
.
It was probably due to the exposure of Logger
as a global variable on configure()
.
Line 208 in a1e40c2
Logger = loggerModule.Logger; |
Lines 134 to 137 in a1e40c2
return { | |
LoggingEvent: LoggingEvent, | |
Logger: Logger | |
}; |
Refactored in 3.0.0
The global variable Logger
has since been removed in 8084e80 which was released as 3.0.0
.
But there was no change to the typings log4js.d.ts
.
Thus, would a single line change from class
to interface
in log4js.d.ts
(a reversal of a1e40c2) be better?
Line 405 in 3582a00
export class Logger { |
- export class Logger {
+ export interface Logger {
Because Logger
should not be used standalone nor exposed. It won't work if log4js
is not configured (which is also handled silently by getLogger()
).
It is wrapped around and should only be retrieved from getLogger()
.
Lines 144 to 154 in 3582a00
function getLogger(category) { | |
if (!enabled) { | |
configure( | |
process.env.LOG4JS_CONFIG || { | |
appenders: { out: { type: 'stdout' } }, | |
categories: { default: { appenders: ['out'], level: 'OFF' } }, | |
} | |
); | |
} | |
return new Logger(category || 'default'); | |
} |
OK. Both methods are available. The key point is keeping them consistent. |
@taozi0818 Thanks! I rebased. |
Fix #1275